Skip to content

fix(sdap): skip unmapped-QFI drop check on UE DL path#158

Closed
alexjiao2021 wants to merge 1 commit into
duranta-project:developfrom
alexjiao2021:fix-ue-sdap
Closed

fix(sdap): skip unmapped-QFI drop check on UE DL path#158
alexjiao2021 wants to merge 1 commit into
duranta-project:developfrom
alexjiao2021:fix-ue-sdap

Conversation

@alexjiao2021

@alexjiao2021 alexjiao2021 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Ping UE failed with 3rd-party gNB, UE will report error as below:
[SDAP] Dropping UL SDAP PDU with unmapped QFI=5 (ue=0 pdu_session=1 drb=2)

gNB has configuration for SDAP as below:

                                drb-ToAddModList: 1 item
                                    Item 0
                                        DRB-ToAddMod
                                            cnAssociation: sdap-Config (1)
                                                sdap-Config
                                                    pdu-Session: 1
                                                    sdap-HeaderDL: absent (1)
                                                    sdap-HeaderUL: present (0)
                                                    ..1. .... defaultDRB: True
                                                    mappedQoS-FlowsToAdd: 1 item
                                                        Item 0
                                                            QFI: 2
                                            drb-Identity: 2

Complete gNB RRC log in pcap format:
nr_rrc.zip
UE log:
nr_ue_ping_fail.log

This PR fixed the issue, details are in commit message.

@alexjiao2021 alexjiao2021 requested a review from GuidoCasati June 10, 2026 05:21
@alexjiao2021 alexjiao2021 added the nrUE Run only 5G UE tests (physims included, LDPC tests excluded). label Jun 10, 2026
@alexjiao2021 alexjiao2021 added this to the REVIEW_CAN_START milestone Jun 10, 2026
@alexjiao2021
fix(sdap): skip unmapped-QFI drop check on UE DL path
4dadeed 
When enable_sdap is true but the DL SDAP header is absent
(sdap_dl_rx == false), the RX entity still reads the first byte
of the buffer as a QFI. On the UE side that byte belongs to the
IP payload, so the qfi2drb_table lookup returns SDAP_NO_MAPPING_RULE
and the packet was silently dropped, causing ping failures.

The drop guard is only meaningful on the gNB UL path, guard it
with is_gnb so the UE DL path is unaffected.

Signed-off-by: Alex Jiao <alex.jiao@keysight.com>
@durantabot

durantabot commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

CI Build: #184 | Failed on the following stages:

@durantabot

durantabot commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

CI Build: #185 | Failed on the following stages:

@GuidoCasati

GuidoCasati commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Hi @alexjiao2021, thanks for reporting this.

We have a similar scenario in https://gitlab.eurecom.fr/oai/cn5g/oai-cn5g-upf/-/work_items/36 in the CN to gNB N3 path: the simple-switch UPF sends DL GTP-U with default qfi = 0x05 until an UL packet reclassifies the session.

In this case, it also happen that QFI=5 is received but for a different reason: gNB sends DL without SDAP header, the UE SDAP sees raw IPv4 SDUs and QFI=5 comes from IPv4 header byte 0x45.

This is actually a bug, since entity->enable_sdap, is true if either UL or DL header is present, and your capture shows that it can be not the case since we have UL header only.

However, the fix is not enough, as it would still parse IP header as an SDAP header, so we have bogus qfi, and also parsing should be done only when RRC says header is present. I'll think about a more proper fix, which I believe should do the header parsing on a per-direction role.

@rorsc rorsc mentioned this pull request Jun 10, 2026
Merged
@alexjiao2021

alexjiao2021 commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

However, the fix is not enough, as it would still parse IP header as an SDAP header, so we have bogus qfi, and also parsing should be done only when RRC says header is present. I'll think about a more proper fix, which I believe should do the header parsing on a per-direction role.

Thanks @GuidoCasati , once you have a more proper fix ready, I'll close this PR.

@GuidoCasati GuidoCasati mentioned this pull request Jun 30, 2026
Open
@GuidoCasati

GuidoCasati commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

However, the fix is not enough, as it would still parse IP header as an SDAP header, so we have bogus qfi, and also parsing should be done only when RRC says header is present. I'll think about a more proper fix, which I believe should do the header parsing on a per-direction role.

Thanks @GuidoCasati , once you have a more proper fix ready, I'll close this PR.

hi @alexjiao2021, could you please test #249 and see if it solve the issue?

@alexjiao2021

alexjiao2021 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

However, the fix is not enough, as it would still parse IP header as an SDAP header, so we have bogus qfi, and also parsing should be done only when RRC says header is present. I'll think about a more proper fix, which I believe should do the header parsing on a per-direction role.

Thanks @GuidoCasati , once you have a more proper fix ready, I'll close this PR.

hi @alexjiao2021, could you please test #249 and see if it solve the issue?

Yes, the issue is solved by PR #249, I will close this one.

GuidoCasati added a commit to GuidoCasati/openairinterface5g that referenced this pull request Jul 9, 2026
@GuidoCasati
fix (sdap): select RX/TX SDAP headers on per-DRB RRC role
1edb0a0 
A DRB may have an uplink SDAP header but no downlink header (valid TS 38.331).
The old entity->enable_sdap flag was true if either direction had a header, so UE
downlink wrongly read byte 0 as QFI. IPv4 payloads start with 0x45 -> bogus QFI 5
and dropped traffic before the TUN (PR !158).

Header presence must follow TS 37.324 §5.2.1/§5.2.2 per DRB and direction, not a
single PDU-session-wide enable-sdap.

Also, store the default DRB as a `qfi2drb_t` mapping in the SDAP entity and use
it directly for fallback paths.

Changes:
- Add nr_sdap_drb_lookup() to return first qfi2drb_table row for a DRB
- nr_sdap_rx_entity(): parse QFI only when the receiving direction has a header,
  drop if the DRB has no table row
- nr_sdap_tx_entity(): UL/DL TX header bits from qfi2drb_map() (drb_id + entity_role),
  including default-DRB fallback
- nr_sdap_qfi2drb(): const qfi2drb_t *, remove SDAP_MAP_RULE_EMPTY
- nr_sdap_map_ctrl_pdu(): handle NULL from qfi2drb_map
- nr_sdap_qfi2drb_map_update(): sync entity_role on
  all QFIs for the DRB. At most one DRB when both headers absent.
- nr_sdap_entity_update_qos_flows(): keep per-DRB role from nr_sdap_drb_lookup(),
  skip update when DRB has no row
- Cleanup entity->enable_sdap
- nr_sdap.c: remove TUN-thread QFI drop, tx_entity owns unmappable-QFI handling
- replace default_drb with qfi2drb_t default_drb in SDAP entity and init
  default DRB id+role at SDAP entity creation

Refs:
- PR duranta-project#158
- TS 37.324 §5.2.1, §5.2.2, §6.2.2.1
- TS 38.331 SDAP-Config (sdap-HeaderUL / sdap-HeaderDL)

Signed-off-by: Guido Casati <guido.casati@openairinterface.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuidoCasati GuidoCasati Awaiting requested review from GuidoCasati

Assignees

No one assigned

Labels

nrUE Run only 5G UE tests (physims included, LDPC tests excluded).

Projects

None yet

Milestone

REVIEW_IN_PROGRESS

Development

Successfully merging this pull request may close these issues.

3 participants

@alexjiao2021 @durantabot @GuidoCasati